Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

remove unstable git archival config #1522

Merged
merged 2 commits into from
Feb 9, 2024
Merged

remove unstable git archival config #1522

merged 2 commits into from
Feb 9, 2024

Conversation

zeke
Copy link
Member

@zeke zeke commented Feb 8, 2024 •
edited
Loading

This PR is an attempt to change our git configuration to prevent unintended changes in checksums as we create new releases.

From @Bo98 in Homebrew/homebrew-core#162013 (comment)

The %D format used in

cog/python/.git_archival.txt

Line 4 in a81c7a4

ref-names: $Format:%D$
is not a stable format and can change depending on HEAD.

ref-names looks like it is actually only necessary to have if using a Git older than 2.32 - describe-name is used instead on modern Git. I'm not sure if that's a concern for cog or not, but removing ref-names could be the fix here. An alternative would be to upload your own stable source archive on releases.

For the record, I Have No Idea What I'm Doing here. 🐶 Not sure if this is the right approach, and just trying to unblock our Homebrew releases. Relying on smarter people to weigh in on whether this is the right change. 🙏🏼

Resolves #1518

@zeke zeke mentioned this pull request Feb 8, 2024
Merged
6 tasks
@stefanb
Copy link

stefanb commented Feb 9, 2024 •
edited
Loading

Is it expected that this change will only affect the hashes of the future cog release archives or also the existing ones (eg current last release 0.9.4)?

cc @nickstenning , the author of the changed lines in d2726e0

@nickstenning
Copy link
Member

nickstenning commented Feb 9, 2024 •
edited
Loading

I'm not sure I understand what has happened here. I get that the export-subst configuration produces output that depends on what HEAD points to, but what I don't understand is how the source tarball actually changed?

Did it change because GitHub changed how the tarballs are generated, i.e. they previously were on main when they generate the export, but now they check out the tag itself?

Edit: looks like it's the opposite. Previously the tarball was generated with HEAD -> main and with working copy from the tag, and now they're on main with working copy from main.

@nickstenning
Copy link
Member

Is it expected that this change will only affect the hashes of the future cog release archives or also the existing ones (eg current last release 0.9.4)?

As far as I can tell it's the export-subst configuration at the relevant treeish that is used during archive, and not the one checked out, so changing this should only change the hashes of future source tarballs.

@nickstenning
Move .git_archival to package root
ee83adf 
The code in setuptools_scm [requires][1] that the .git_archival.txt file
lives at the package root. The package root was originally python/ but
that was changed by d86fb17 and this got left behind.

[1]: https://github.com/pypa/setuptools_scm/blob/a23d5fe61d9dc421666d5f23334d454426ad311c/src/setuptools_scm/git.py#L334
@nickstenning
Copy link
Member

I pushed a commit to this branch to move .git_archival.txt to the root of the repository. It turns out that the relevant setuptools_scm code (to generate the right version for the Python package when installed from an archive tarball) has been broken since d86fb17.

nickstenning
nickstenning approved these changes Feb 9, 2024
View reviewed changes
Copy link
Member

@nickstenning nickstenning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nickstenning
Copy link
Member

I'm going to go ahead and get this merged. Thank you for picking it up @zeke :)

@nickstenning nickstenning merged commit 181be52 into main Feb 9, 2024
14 checks passed
@nickstenning nickstenning deleted the remove-unstable-git-archival-config branch February 9, 2024 18:09
@yorickvP yorickvP mentioned this pull request Feb 16, 2024
Closed
@BrewTestBot BrewTestBot mentioned this pull request Mar 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickstenning nickstenning nickstenning approved these changes

@mattt mattt Awaiting requested review from mattt

@technillogue technillogue Awaiting requested review from technillogue

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

checksum mismatch for 0.9.4 release
3 participants
@zeke @stefanb @nickstenning